Add code quality issue listing to the CLI - #130
Conversation
Add 'corgea list --code-quality' (alias --quality) to list code quality findings, mirroring --issues but hitting the code quality endpoints (/scan/<id>/issues/quality and /issues/code-quality). - Add get_quality_issues() to the API client. - Deserialize the new 'type' discriminator on issues (optional). - Make --issues, --sca-issues, and --code-quality mutually exclusive. - Add a deserialization test for code quality issue responses. Co-authored-by: ibrahim <ibrahim@corgea.com>
Code quality and security issues come from separate endpoints, so the CLI does not need a 'type' discriminator on the Issue struct. Keeps the CLI in sync with the API response, which no longer emits the field. Co-authored-by: ibrahim <ibrahim@corgea.com>
CI runs clippy with -D warnings; adding the --code-quality flag pushed list::run to 8 arguments (limit 7). Allow the lint here, matching the existing pattern used in blast.rs and deps/ecosystems/evaluate.rs. Co-authored-by: ibrahim <ibrahim@corgea.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Production review on c72c3d6 (vs main). Three merge blockers below: CQ listing incorrectly inherits security blocking-rule failure modes, the new HTTP client ignores status codes during a companion-API rollout, and the only new test does not lock the endpoint contract.
Also confirm the doghouse companion (PR body cites #1715) is deployed before shipping this CLI — the feature hard-depends on those endpoints being healthy.
Sent by Cursor Automation: pr-flow
| *page_size, | ||
| scan_id.clone(), | ||
| ) { | ||
| } else if *issues || *code_quality { |
There was a problem hiding this comment.
--code-quality inherits security blocking-rule side effects (including hard failure).
Folding CQ into the *issues branch means corgea list --code-quality --scan-id ... still runs check_blocking_rules (lines 162-189) and exits 1 on any failure, even after get_quality_issues already succeeded.
Impact: A blocking-rules API blip (or security-only block: true) makes CQ listing unusable, or adds empty Blocking columns driven by non-CQ findings.
Fix: Gate enrichment on security listing only, e.g. if scan_id.is_some() && !*code_quality { ... }. Or, if CQ rows can legitimately be blocked, soft-fail the enrichment (log + continue) and set render_blocking_rules only when at least one returned CQ issue id is in the blocking map. Add a regression test for --code-quality --scan-id that stubs blocking-rules failure and asserts the CQ table/JSON still prints.
| let response = match client.get(&url).send() { | ||
| Ok(res) => { | ||
| check_for_warnings(res.headers(), res.status()); | ||
| res | ||
| } | ||
| Err(e) => return Err(format!("Failed to send request: {}", e).into()), | ||
| }; | ||
| let response_text = response.text()?; |
There was a problem hiding this comment.
New client ignores HTTP status; non-JSON errors become opaque parse/auth failures.
get_quality_issues always reads the body and serde_json::from_strs with no status().is_success() check (unlike get_sca_issues in this same file, which maps 404/non-2xx explicitly).
Impact: During companion rollout (or any 401/403/404/500 HTML/JSON error body), users get Failed to parse response then the generic check-token/connection path in list.rs, and the 404 string match for scan/project-does-not-exist never fires. A wrong path (/issues/quality vs /issues/code-quality) is easy to miss.
Fix: Mirror get_sca_issues: inspect status before parse; map 404/401/5xx to distinct errors that list.rs can surface. Prefer client.get(&endpoint).query(...) so project names are encoded and page_size cannot be appended as &page_size= without a ? when page is None.
| fn deserializes_code_quality_issue_response() { | ||
| // Code quality issues carry a free-form classification label (no CWE) and | ||
| // must deserialize into the same Issue struct used for security issues. | ||
| let body = r#"{ | ||
| "status": "ok", | ||
| "page": 1, | ||
| "total_pages": 1, | ||
| "total_issues": 1, | ||
| "issues": [ | ||
| { | ||
| "id": "11111111-1111-1111-1111-111111111111", | ||
| "urgency": "ME", | ||
| "created_at": "2026-01-01T00:00:00Z", | ||
| "status": "open", | ||
| "classification": { | ||
| "id": "Maintainability", | ||
| "name": "Maintainability", | ||
| "description": null | ||
| }, | ||
| "location": { | ||
| "file": {"name": "app.py", "language": "python", "path": "app/app.py"}, | ||
| "project": {"name": "proj", "branch": "main", "git_sha": "abc"}, | ||
| "line_number": 20 | ||
| }, | ||
| "auto_triage": {"false_positive_detection": {"status": "valid"}}, | ||
| "auto_fix_suggestion": {"status": "no_fix"} | ||
| } | ||
| ] | ||
| }"#; | ||
|
|
||
| let parsed: ProjectIssuesResponse = | ||
| serde_json::from_str(body).expect("should parse code quality response"); |
There was a problem hiding this comment.
Test passes without exercising the new behavior (endpoint contract / CLI wiring).
deserializes_code_quality_issue_response only round-trips a hand-written JSON fixture into ProjectIssuesResponse. It would still pass if:
- scan path were
/scan/{id}/issues/code-quality(or project path/issues/quality) — the PR uses asymmetric/scan/.../issues/qualityvs/issues/code-qualitywith zero assertion --issues/--sca-issues/--code-qualitymutual exclusion regressed--code-quality --scan-idwrongly calledget_scan_issues
Impact: False confidence for a net-new production API surface.
Fix: Add a unit/integration test that builds the request URL (or uses a mock HTTP server) and asserts both endpoint variants + query params; add a clap/CLI test that --issues --code-quality exits 1 and that --code-quality selects get_quality_issues.
corgea list used the current directory basename as the project key, while corgea scan stores projects under determine_project_name (which prefers the Git remote repository name). This caused 'Project not found' when the checkout directory differed from the repo name, including Git worktrees. Use the same determine_project_name helper for list so the lookup key matches what scans are stored under. Applies to --issues, --sca-issues, --code-quality, and the default scan listing. Co-authored-by: ibrahim <ibrahim@corgea.com>
- Add a --project-name flag to 'corgea list', matching 'corgea scan', and thread it through determine_project_name so users can override the resolved project key explicitly. - Gate blocking-rules enrichment to security listings only. Previously 'corgea list --code-quality --scan-id' ran check_blocking_rules and hard-exited on any failure (even after the CQ fetch succeeded), and could render Blocking columns driven by non-CQ findings. Co-authored-by: ibrahim <ibrahim@corgea.com>
Resolves list/main conflicts on top of the COR-1577 project resolution work: the code quality listing now goes through ProjectSelector and resolve_project_or_exit like every other list mode. Co-authored-by: Ibrahim Rahhal <ibrahim.rahhal3636@gmail.com>
Adds end-to-end coverage for the two documented code quality routes, the project resolution they share with --issues, the blocking-rules gate, and the 404 mapping, plus a unit test pinning the request paths and query. Documents the flag in the agent skill. Co-authored-by: Ibrahim Rahhal <ibrahim.rahhal3636@gmail.com>
5 participants
Summary
The CLI only exposed security (SAST) and SCA findings — code quality findings were not reachable at all. This adds first-class support for listing them, against the endpoints documented at
docs.corgea.app:GET /api/v1/issues/code-quality(project-scoped) andGET /api/v1/scan/{scan_id}/issues/quality(scan-scoped).Changes
corgea list --code-quality(alias--quality, short-q): lists code quality findings for the current project or a specific--scan-id, in the same table /--jsonformats as--issues. TheCategorycolumn shows the code quality label (e.g.Maintainability) in place of a CWE id.utils/api.rs: newget_quality_issues(). The endpoint and query are built byquality_issues_request(), which sends the project throughqueryrather than string interpolation (project names can contain&/?/#), and the HTTP status is read before the body so a bare 404 surfaces as a missing scan rather than a parse failure.ProjectSelector/resolve_project_or_exit(), exactly like--issuesand the default scan listing, so--project-name,--repo, and repo-URL auto-resolution all behave identically (COR-1577).--issues,--sca-issues, and--code-qualityare mutually exclusive.list --code-quality --scan-idwould still callcheck_blocking_rules(hard-exiting on any failure even after the code quality fetch succeeded) and could render Blocking columns driven by non-code-quality findings.--code-qualitydocumented inskills/corgea/SKILL.md.Testing
tests/list_code_quality.rs— end-to-end coverage over a recording stub: both documented request targets are asserted,--quality/-qreach the same route, the canonical project resolved from the git remote scopes the request, the project name is percent-encoded,--code-quality --scan-idnever dialscheck_blocking_rules(unstubbed, so reaching it would exit 1) and never dials/projects, a 404 reports the missing scan instead of a parse failure, and every pair of issue-kind flags is rejected.src/utils/api.rs— unit tests pinning the two request paths plus their query params, and the code quality response deserialization (label classification with a null description)../harness ci— clippy (strict), format check, dep audit, 617 tests and the coverage gate all pass.Example